Adding helper methods for cluster share and unshare #2729
Conversation
ak-px
force-pushed
the
ak/topic/cluster_share_master
branch
2 times, most recently
from
78f0e60 to
99964cd
Compare
ak-px
force-pushed
the
ak/topic/cluster_share_master
branch
from
99964cd to
0e81a54
Compare
tests/backup_helper.go
Outdated
| } | ||
|
|
||
| // ShareClusterWithValidation shares a cluster with users and groups and optionally shares existing backups and validates the share. | ||
| func ShareClusterWithValidation(ctx context1.Context, clusterName string, clusterUid string, users []string, groups []string, shareClusterBackups bool) (*api.ShareClusterResponse, error) { |
There was a problem hiding this comment.
Can you re Use ShareCluster function and avoid duplicate code for Sharing the cluster.
10522 - 10540 is duplicate of function ShareCluster Please reuse
tests/backup_helper.go
Outdated
| } | ||
|
|
||
| // UnShareClusterWithValidation unshares a cluster with users and groups and validates the unshare. | ||
| func UnShareClusterWithValidation(ctx context1.Context, clusterName string, clusterUid string, users []string, groups []string) (*api.UnShareClusterResponse, error) { |
There was a problem hiding this comment.
Same here, please avoid duplicate code and reuse UnshareCluster
| Uid: clusterUid, | ||
| } | ||
|
|
||
| // Inspect cluster to retrieve collaborators |
There was a problem hiding this comment.
These are functional level validation and more specificly this seems to me like a unit test case. than system test. Its good to have but will add test run time.
I would suggest to validate access through Backup Operations such as
Create Backup by shared user on the cluster
Create backup by user part of the group to which the cluster is shared.
There was a problem hiding this comment.
These are part of the helper method validation. All other backup-related CRUD operations will be validated at the test case level. This serves as a basic validation for the system tests.
| } | ||
|
|
||
| // Validate that the entire list matches | ||
| if !AreStringSlicesEqual(userIds, users) { |
There was a problem hiding this comment.
This will ensure that the fetched list from collaborators doesn't contain any extra entries.
| return | ||
| } | ||
| log.Infof("Inspecting cluster [%s] with user [%s]", clusterName, user) | ||
| _, err = backupDriver.InspectCluster(nonAdminCtx, req) |
There was a problem hiding this comment.
I would prefer createBackup, createBackupSchedule and Create restore on this cluster is better validation .
There was a problem hiding this comment.
All backup-related CRUD operations will be tested at the test case level
| return | ||
| } | ||
|
|
||
| if backupObj.GetBackup().UserBackupshareAccess != api.BackupShare_Restorable { |
There was a problem hiding this comment.
These should be changed to create Restore from backup.
There was a problem hiding this comment.
same as above, This serves as a basic validation to ensure the backup object has Restore access after the cluster is shared. All other CRUD operations will be validated at the test case level.
| Name: backupName, | ||
| } | ||
| log.Infof("Inspecting backup [%s] with user [%s]", backupName, user) | ||
| backupObj, err := backupDriver.InspectBackup(nonAdminCtx, backupInspectRequest) |
There was a problem hiding this comment.
Instead of inspect you should execute Restore from backup.
ak-px
force-pushed
the
ak/topic/cluster_share_master
branch
2 times, most recently
from
36914ba to
f42f4ed
Compare
ak-px
force-pushed
the
ak/topic/cluster_share_master
branch
from
f42f4ed to
8c14afe
Compare
What this PR does / why we need it:
Adding helper methods cluster share and unshare
Which issue(s) this PR fixes (optional)
Closes #PB-7787
#PB-7788
#PB-7789
Special notes for your reviewer:
The methods are not tested, will fix any issue during testcase automation phase.